Conversation
|
Thank you for your contribution! Before we can merge this PR, we need you to sign our Contributor License Agreement. To sign the CLA, please add a comment to this PR with the following text: You only need to sign once. After signing, this check will pass automatically. Troubleshooting
You can retrigger this bot by commenting **recheck** in this Pull Request. Posted by the **CLA Assistant Lite bot**. |
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Greptile OverviewGreptile SummaryThis PR adds an Intercom integration to the extension’s new tab app by wrapping the React tree in an Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant NewTab as NewTab (app entrypoint)
participant Provider as IntercomProvider (React)
participant Intercom as Intercom JS bootstrap
participant iframe as Sandbox iframe (intercom.sandbox)
NewTab->>Provider: Render app
Provider->>Intercom: init(appId)
Intercom->>iframe: create hidden iframe + inject script
iframe->>Intercom: postMessage "intercom-ready"
Provider->>Intercom: update(user/context)
Intercom-->>NewTab: ready/event callbacks
|
| import type { FC, PropsWithChildren } from 'react' | ||
| import { useEffect, useRef, useState } from 'react' | ||
| import { useSessionInfo } from '@/lib/auth/sessionStorage' |
There was a problem hiding this comment.
Sandbox boot may be missed
If sessionInfo resolves before the iframe onLoad fires, the identity sync effect posts intercom:update while iframeWindowRef.current is still null, and there’s no retry after boot. In that scenario, Intercom will boot without user identity until a later session change occurs. Consider sending an intercom:update after intercom:ready (or after handleIframeLoad) when sessionInfo.user is already available.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/agent/lib/intercom/IntercomProvider.tsx
Line: 1:3
Comment:
**Sandbox boot may be missed**
If `sessionInfo` resolves before the iframe `onLoad` fires, the identity sync effect posts `intercom:update` while `iframeWindowRef.current` is still `null`, and there’s no retry after boot. In that scenario, Intercom will boot without user identity until a later session change occurs. Consider sending an `intercom:update` after `intercom:ready` (or after `handleIframeLoad`) when `sessionInfo.user` is already available.
How can I resolve this? If you propose a fix, please make it concise.| const [isMessengerOpen, setIsMessengerOpen] = useState(false) | ||
| const { sessionInfo, isLoading } = useSessionInfo() | ||
|
|
||
| const handleIframeLoad = () => { |
There was a problem hiding this comment.
postMessage accepts any origin
postMessage(..., '*') combined with a sandbox that also posts with '*' means any frame/window can spoof intercom:* messages to this page. Even though the listener checks type, it doesn’t verify event.source === iframeRef.current?.contentWindow (or an expected origin), so a malicious page could toggle state or trigger shutdown/update. Tighten validation to only accept messages from the sandbox iframe window.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/agent/lib/intercom/IntercomProvider.tsx
Line: 27:30
Comment:
**postMessage accepts any origin**
`postMessage(..., '*')` combined with a sandbox that also posts with `'*'` means any frame/window can spoof `intercom:*` messages to this page. Even though the listener checks `type`, it doesn’t verify `event.source === iframeRef.current?.contentWindow` (or an expected origin), so a malicious page could toggle state or trigger shutdown/update. Tighten validation to only accept messages from the sandbox iframe window.
How can I resolve this? If you propose a fix, please make it concise.| parent.postMessage(message, '*') | ||
| } | ||
|
|
||
| function initIntercomStub() { |
There was a problem hiding this comment.
postMessage targetOrigin too broad
The sandbox sends events via parent.postMessage(message, '*'), which allows any embedding origin to receive Intercom state events if this page is ever embedded outside the extension context. Since the parent is expected to be the extension page, pass an explicit targetOrigin (or at least validate event.origin on the parent side and restrict event.source).
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/agent/entrypoints/intercom.sandbox/main.ts
Line: 13:16
Comment:
**postMessage targetOrigin too broad**
The sandbox sends events via `parent.postMessage(message, '*')`, which allows any embedding origin to receive Intercom state events if this page is ever embedded outside the extension context. Since the parent is expected to be the extension page, pass an explicit `targetOrigin` (or at least validate `event.origin` on the parent side and restrict `event.source`).
How can I resolve this? If you propose a fix, please make it concise.
Summary
let's integreate intercom with newtab
Changes
Agent Metadata
Generated by coding-agent v3